Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump zarr reader 0.5.1 #427

Merged
merged 6 commits into from
Jun 27, 2024
Merged

Conversation

will-moore
Copy link
Member

NB: this is on top of @dominikl's PR: #420 following the discussion there, particularly #420 (comment)

@sbesson: I have added the list of jars under omero_client_jars but as I understand it, this will attempt to load those jars from the OME artifactory, not from maven central?

I added a variable maven_central_baseurl: "https://repo1.maven.org/maven2" and I should probably list the jars under a different variable like omero_client_maven_jars but I don't know where to update the logic that would consume that list.

cc @dominikl @jburel

@sbesson
Copy link
Member

sbesson commented Jun 19, 2024

@will-moore I would suggest to keep exactly the logic in the current playbook but change the name of the dictionary e.g. to use zarrreader_jars

    - name: Override lib/server JARs
      become: yes
      get_url:
        url: "{{ artifactory_baseurl }}/{{ item.group }}/{{ item.name }}/{{ item.version }}/{{ item.name }}-{{ item.version }}.jar"
        dest: "{{ omero_common_basedir }}/server/OMERO.server/lib/server/{{ item.name }}.jar"
        force: yes
      with_items: "{{ zarrreader_jars }}"
      notify: restart omero-server

    - name: Override lib/client JARs
      become: yes
      get_url:
        url: "{{ artifactory_baseurl }}/{{ item.group }}/{{ item.name }}/{{ item.version }}/{{ item.name }}-{{ item.version }}.jar"
        dest: "{{ omero_common_basedir }}/server/OMERO.server/lib/client/{{ item.name }}.jar"
        force: yes
      with_items: "{{ zarrreader_jars }}"
      notify: restart omero-server

and in ansible/group_vars/omero_hosts.yml, rename omero_client_jars as zarrreader_jars and add OMEZarrReader 0.5.1 to the list

@will-moore
Copy link
Member Author

@sbesson I don't see how we can have OMEZarrReader in the same list as the other dependencies since they are being loaded from different artifactories - (and the artifactory isn't specified in the list).

E.g. https://artifacts.openmicroscopy.org/artifactory/ome.releases/ome/OMEZarrReader/0.5.1/OMEZarrReader-0.5.1.jar
vv https://repo1.maven.org/maven2/com/amazonaws/aws-java-sdk-s3/1.12.659/aws-java-sdk-s3-1.12.659.jar

@sbesson
Copy link
Member

sbesson commented Jun 20, 2024

If you use the virtual maven repository rather than the ome.releases repository, you should be able to retrieve both artifacts

https://artifacts.openmicroscopy.org/artifactory/maven/ome/OMEZarrReader/0.5.1/
https://artifacts.openmicroscopy.org/artifactory/maven/com/amazonaws/aws-java-sdk-s3/1.12.659/

@will-moore will-moore force-pushed the bump_zarr_reader_0.5.1 branch from 72253b2 to 9e846b4 Compare June 20, 2024 09:27
@will-moore
Copy link
Member Author

@sbesson OK thanks. Changes pushed.

@will-moore
Copy link
Member Author

What's the plan for testing this? Deploy on a pilot or onto idr-next?

@will-moore
Copy link
Member Author

@sbesson Does this look good now? Any way we can test it?

@sbesson
Copy link
Member

sbesson commented Jun 21, 2024

Having not heard from the rest of the IDR team on this, I think the available options deployment wise are :

  • use test122 i.e. current idr-testing.openmicroscopy.org
  • use test122b i.e. current idr-next.openmicroscopy.org
  • spin up prod122 and assign it to idr-next.openmicroscopy.org

I would suggest we make use the upcoming Monday meeting to make the decision

@will-moore
Copy link
Member Author

Seb has deployed this on idr-next for testing...
Installed omero-mkngff and ran sql scripts for idr0004 as described at IDR/idr-metadata#696 (comment)

Viewing in webclient to trigger memo generation, then viewing an image from first plate:
Blitz logs show blosc is missing:

2024-06-24 16:46:00,102 ERROR [        ome.services.util.ServiceHandler] (l.Server-4) java.lang.Error:  Wrapped Exception: (java.lang.NoClassDefFoundError):
Could not initialize class org.blosc.IBloscDll
java.lang.NoClassDefFoundError: Could not initialize class org.blosc.IBloscDll
	at com.bc.zarr.CompressorFactory$BloscCompressor.cbufferSizes(CompressorFactory.java:352)
	at com.bc.zarr.CompressorFactory$BloscCompressor.uncompress(CompressorFactory.java:338)
	at com.bc.zarr.chunk.ChunkReaderWriterImpl_Short.read(ChunkReaderWriterImpl_Short.java:55)
	at com.bc.zarr.ZarrArray.read(ZarrArray.java:278)
	at com.bc.zarr.ZarrArray.read(ZarrArray.java:255)
	at loci.formats.services.JZarrServiceImpl.readBytes(JZarrServiceImpl.java:241)

@sbesson
Copy link
Member

sbesson commented Jun 24, 2024

To be specific, I deployed the combination of #427, #424 and #423 against test122b.

The error above suggests we are missing the blosc dependency which brings us back to #420 (comment). Could you update this PR to bump the role version and I'll redeploy tomorrow morning.

@will-moore
Copy link
Member Author

@sbesson Sorry, I'm not sure where/how to "update this PR to bump the role version".

@will-moore
Copy link
Member Author

We still have this in deployment/ansible/requirements.yml

- src: ome.omero_server
  version: 6.0.0

Is that what you meant?
In the discussion on that at #420 (review) you say "the plan for prod122 is to bump OMERO.server to the just released 5.6.11"
Which seems to mean installing an older version rather than bumping the version?

@sbesson
Copy link
Member

sbesson commented Jun 24, 2024

These are two independent versions. #420 (comment) is referring to the version of the Ansible role as the blosc installation was introduced in ome/ansible-role-omero-server#75. #420 (review) is referring to the value of idr_omero_server_release which specifies the version of the OMERO.server binary deployed by the role and is already updated by this PR.

@will-moore
Copy link
Member Author

@sbesson understood now, thanks. Bumped the role version in 8caf45e

@sbesson sbesson self-requested a review June 24, 2024 22:05
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executed the idr-01-install-idr.yml playbook with the latest changes against the test122b environment

TASK [ome.omero_server : server package | install redhat blosc] ************************************************************************************
changed: [test122b-omeroreadwrite]
...

TASK [ome.omero_server : server package | install redhat blosc] ************************************************************************************
changed: [test122b-omeroreadonly-1]
changed: [test122b-omeroreadonly-4]
changed: [test122b-omeroreadonly-3]
changed: [test122b-omeroreadonly-2]

PLAY RECAP *****************************************************************************************************************************************
test122b-database          : ok=46   changed=0    unreachable=0    failed=0    skipped=22   rescued=0    ignored=0   
test122b-management        : ok=15   changed=0    unreachable=0    failed=0    skipped=2    rescued=0    ignored=0   
test122b-omeroreadonly-1   : ok=150  changed=6    unreachable=0    failed=0    skipped=48   rescued=0    ignored=0   
test122b-omeroreadonly-2   : ok=150  changed=6    unreachable=0    failed=0    skipped=48   rescued=0    ignored=0   
test122b-omeroreadonly-3   : ok=150  changed=6    unreachable=0    failed=0    skipped=48   rescued=0    ignored=0   
test122b-omeroreadonly-4   : ok=150  changed=6    unreachable=0    failed=0    skipped=48   rescued=0    ignored=0   
test122b-omeroreadwrite    : ok=145  changed=1    unreachable=0    failed=0    skipped=47   rescued=0    ignored=0   
test122b-proxy             : ok=81   changed=0    unreachable=0    failed=0    skipped=22   rescued=0    ignored=0   
test122b-searchengine      : ok=23   changed=0    unreachable=0    failed=0    skipped=5    rescued=0    ignored=0   

From a deployment perspective, the proposed changes make sense. Once tested functionally, we should be able to use this to create prod122.

@will-moore
Copy link
Member Author

@sbesson I don't know if this is expected to be fixed now, but I'm still seeing Blosc errors on test122b-omeroreadwrite when trying to view NGFF data (idr0004)?

@sbesson
Copy link
Member

sbesson commented Jun 25, 2024

Definitely not expected but one thing that comes to mind is that the playbook didn't restart the omero-server service (since nothing changed). Can you try running sudo systemctl restart omero-server and see if the blosc issue persists?

@will-moore
Copy link
Member Author

Great - yes that fixed it, Thanks. I didn't need to do that on e.g. omeroreadonly-3 after I restarted omeroreadwrite server.
Is that effectively the same as sudo service omero-server restart?

@sbesson
Copy link
Member

sbesson commented Jun 25, 2024

Great - yes that fixed it, Thanks. I didn't need to do that on e.g. omeroreadonly-3 after I restarted omeroreadwrite server.

These might have been restarted as per the changed=6 above. Only the omeroreadwrite server has not. I don't think it's worth investigating why this is different at this stage.

Is that effectively the same as sudo service omero-server restart?

Yes exactly.

Copy link
Member Author

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working for me now, allowing viewing of NGFF data on idr-next.

Can't approve my own PR but 👍

@sbesson sbesson requested a review from jburel June 26, 2024 11:02
@jburel jburel merged commit e90627c into IDR:master Jun 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants